[JIT] X64 - Centralize peephole optimization for removing redundant mov instructions
#85780
Conversation
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsDescriptionDepending on what we do for #85734 , we need to counter some of the regressions that may occur as a result of not removing
|
TIHan
changed the title
movzx and movsx instructions
…e optimization to IsRedundantMov.
TIHan
changed the title
movzx and movsx instructionsmov instructions
TIHan
changed the title
mov instructionsmov instructions
|
/azp run runtime-coreclr gcstress0x3-gcstress0xc |
|
/azp run Fuzzlyn |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Fuzzlyn |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@dotnet/jit-contrib this is ready. PTAL @BruceForstall - Diffs All diff improvements, and we get a TP win for x64. |
|
Thinking about it, I'm not sure why there are TP regressions for x86 for MinOpts. These optimizations only kick in when optimizations are on. |
src/coreclr/jit/emitxarch.cpp
Outdated
| { | ||
| result = (id->idOpSize() <= size); | ||
| } | ||
| #ifdef TARGET_64BIT |
There was a problem hiding this comment.
On 32-bit will EA_4BYTE ever be passed? If not, we don't technically don't need the ifdef?
There was a problem hiding this comment.
size can be EA_4BYTE on 32-bit - ifdef'ing this only for 64-bit just does less work on 32-bit.
| result = (id->idOpSize() <= size); | ||
| } | ||
| #ifdef TARGET_64BIT | ||
| // movsx/movsxd always sign extends to 8 bytes. W-bit is set. |
There was a problem hiding this comment.
Why does the W-bit matter here?
There was a problem hiding this comment.
W-bit on the encoding is what makes movsx and movsxd sign-extend to 8 bytes. We always default to using the W-bit. Technically, we could have a movsx or movsxd instruction not set the W-bit which would not sign-extend to 8 bytes (that would be a problem and make this optimization not safe), but we never actually emit those instructions that way.
There was a problem hiding this comment.
This function cares about instructions and sizes, but not encoding, though.
There was a problem hiding this comment.
The comment W-bit is set itself is just meant to inform the reason why this optimization is safe to do. It was already there before.
ghost
added
needs-author-action
|
@BruceForstall This is ready again. |
|
@BruceForstall This is ready again. The optimizations only happen for X64, which was the case before, so no TP regressions for X86. |
Description
This centralizes checking for redundant
movinstructions.Depending on what we do for #85734 , we need to counter some of the regressions that may occur as a result of not removing
CASTnodes.This also includes more peephole optimizations for
movzx,movsxandmovsxd.